Remove unwanted inner try block#2741
Conversation
| } | ||
| } catch (Exception e) { | ||
| DebugPlugin.log(e); | ||
| if (attr.startsWith(MULTI_LAUNCH_CONSTANTS_PREFIX)) { |
There was a problem hiding this comment.
What kind of exception might happen? Do you really want the whole loop to terminate if there is an exception?
There was a problem hiding this comment.
Since neither startsWith() nor removeAttribute() declares any exceptions, I initially thought the inner try/catch could be removed
There was a problem hiding this comment.
It all looks a little overly cautious given that nothing declares any thrown exceptions. It looks like it's always been there so there's no real accounting for why either of those try blocks are there, neither the inner nor the outer. Maybe it happened during some initial implementation and was then never cleaned up. I'm not sure what to say about that. 😞
There was a problem hiding this comment.
I checked other references of removeAttribute(), and they don't have a similar try/catch. Since neither API declares any exceptions, the inner try/catch may just be leftover from an older implementation.
There was a problem hiding this comment.
Do other places have the outer try block? It all looks a bit bogus...
There was a problem hiding this comment.
Only place I found a try block is in org.eclipse.jdt.debug.ui.launchConfigurations.JavaDependenciesTab.performApply(ILaunchConfigurationWorkingCopy) but that is only for catching CoreException (which is for org.eclipse.debug.core.ILaunchConfiguration.getAttribute(String, boolean) )
everywhere else is try-catch free..
SougandhS
force-pushed
the
RemoveUnwantedTryCatchBlock
branch
from
7cd6483 to
d3a1ab6
Compare
merks
left a comment
merks
left a comment
There was a problem hiding this comment.
It’s okay then. Thanks for checking.
Thanks! |
SougandhS
force-pushed
the
RemoveUnwantedTryCatchBlock
branch
from
d3a1ab6 to
e21f968
Compare
2 participants
No description provided.